Fix update document with knnn_vector size not matching issue #208
Conversation
Signed-off-by: zane-neo <[email protected]>
zane-neo
requested review from
heemin32,
navneet1v,
VijayanB,
vamshin,
jmazanec15,
naveentatikonda,
junqiu-lei,
martin-gaievski,
sean-zheng-amazon,
model-collapse,
wujunshen,
ylwu-amzn and
jngz-es
as code owners
|
@zane-neo before PR can be reviewed, can you check why gradle checks are failing? |
Signed-off-by: zane-neo <[email protected]>
Added commons-lang3 back, in my local even I removed this dependency the project can be compiled successfully, and adding back this can cause jar hell issue when building neural-search plugin, but it seems have to add it back to pass the build. |
|
@zane-neo the build and test gradle check is still failing. Please fix them. |
Signed-off-by: zane-neo <[email protected]>
Signed-off-by: zane-neo <[email protected]>
Signed-off-by: zane-neo <[email protected]>
@navneet1v opensearch core changed some classes breaking the code build, I've created a PR to fix this issue in k-NN repo: opensearch-project/k-NN#958, please help take a look. Thanks. |
Signed-off-by: zane-neo <[email protected]>
Signed-off-by: zane-neo <[email protected]>
Signed-off-by: zane-neo <[email protected]>
| @@ -119,7 +117,7 @@ void appendVectorFieldsToDocument(IngestDocument ingestDocument, Map<String, Obj | |||
| Objects.requireNonNull(vectors, "embedding failed, inference returns null result!"); | |||
| log.debug("Text embedding result fetched, starting build vector output!"); | |||
| Map<String, Object> textEmbeddingResult = buildTextEmbeddingResult(knnMap, vectors, ingestDocument.getSourceAndMetadata()); | |||
| textEmbeddingResult.forEach(ingestDocument::appendFieldValue); | |||
| textEmbeddingResult.forEach(ingestDocument::setFieldValue); | |||
There was a problem hiding this comment.
can we rename the function from appendVectorFieldsToDocument to setVectorFieldsToDocument, as we are not appending the values
…cument Signed-off-by: zane-neo <[email protected]>
Signed-off-by: zane-neo <[email protected]>
|
Build is failing caused by opensearch core breaking change, and is fixed by k-NN team in this PR: opensearch-project/k-NN#963. |
zane-neo
added
the
backport 2.x
|
The backport to To backport manually, run these commands in your terminal: # Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-2.x 2.x
# Navigate to the new working tree
cd .worktrees/backport-2.x
# Create a new branch
git switch --create backport/backport-208-to-2.x
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 95993717e59b31bcd0facc76b0c702aa6857b8f8
# Push it to GitHub
git push --set-upstream origin backport/backport-208-to-2.x
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-2.xThen, create a pull request where the |
…rch-project#208) * Fix update document with knnn_vector size not matching issue Signed-off-by: zane-neo <[email protected]> * Add apache common lang3 back to fix PR gradle build failure Signed-off-by: zane-neo <[email protected]> * Fix PR check failure Signed-off-by: zane-neo <[email protected]> * Fix PR check failure Signed-off-by: zane-neo <[email protected]> * Fix PR check jar hell failure Signed-off-by: zane-neo <[email protected]> * Add apache common lang back to fix the PR check Signed-off-by: zane-neo <[email protected]> * Fix register model group failure in IT Signed-off-by: zane-neo <[email protected]> * format code Signed-off-by: zane-neo <[email protected]> * Rename the method appendVectorFieldsToDocument to setVectorFieldsToDocument Signed-off-by: zane-neo <[email protected]> * format code Signed-off-by: zane-neo <[email protected]> --------- Signed-off-by: zane-neo <[email protected]> (cherry picked from commit 9599371)
|
Looks like this PR directly merged to 2.x branch instead of main, I am adding 2.9 release tag for the PR and adding to 2.9 release note. @zane-neo @ylwu-amzn |
junqiu-lei
added
v2.9.0
Bug Fixes
|
@zane-neo this PR also needs to be raised on main branch. Please going forward follow the below practice. |
Description
This PR fixed the issue when updating a document with knn_vector field, the update fails because the vector dimension is not matching the required dimension.
Issues Resolved
#207
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.